[libc++] default some special members in map and set #147081
Conversation
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesWe don't actually do anything special in these special member functions, so we can just Full diff: https://github.com/llvm/llvm-project/pull/147081.diff 2 Files Affected:
diff --git a/libcxx/include/map b/libcxx/include/map
index 6c271392911dc..2070c91cfc46d 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -972,31 +972,15 @@ public:
_LIBCPP_HIDE_FROM_ABI map(const map& __m) : __tree_(__m.__tree_) { insert(__m.begin(), __m.end()); }
- _LIBCPP_HIDE_FROM_ABI map& operator=(const map& __m) {
-# ifndef _LIBCPP_CXX03_LANG
- __tree_ = __m.__tree_;
-# else
- if (this != std::addressof(__m)) {
- __tree_.clear();
- __tree_.value_comp() = __m.__tree_.value_comp();
- __tree_.__copy_assign_alloc(__m.__tree_);
- insert(__m.begin(), __m.end());
- }
-# endif
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI map& operator=(const map& __m) = default;
# ifndef _LIBCPP_CXX03_LANG
- _LIBCPP_HIDE_FROM_ABI map(map&& __m) noexcept(is_nothrow_move_constructible<__base>::value)
- : __tree_(std::move(__m.__tree_)) {}
+ _LIBCPP_HIDE_FROM_ABI map(map&& __m) noexcept(is_nothrow_move_constructible<__base>::value) = default;
_LIBCPP_HIDE_FROM_ABI map(map&& __m, const allocator_type& __a);
- _LIBCPP_HIDE_FROM_ABI map& operator=(map&& __m) noexcept(is_nothrow_move_assignable<__base>::value) {
- __tree_ = std::move(__m.__tree_);
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI map& operator=(map&& __m) noexcept(is_nothrow_move_assignable<__base>::value) = default;
_LIBCPP_HIDE_FROM_ABI map(initializer_list<value_type> __il, const key_compare& __comp = key_compare())
: __tree_(__vc(__comp)) {
@@ -1659,31 +1643,16 @@ public:
insert(__m.begin(), __m.end());
}
- _LIBCPP_HIDE_FROM_ABI multimap& operator=(const multimap& __m) {
-# ifndef _LIBCPP_CXX03_LANG
- __tree_ = __m.__tree_;
-# else
- if (this != std::addressof(__m)) {
- __tree_.clear();
- __tree_.value_comp() = __m.__tree_.value_comp();
- __tree_.__copy_assign_alloc(__m.__tree_);
- insert(__m.begin(), __m.end());
- }
-# endif
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI multimap& operator=(const multimap& __m) = default;
# ifndef _LIBCPP_CXX03_LANG
- _LIBCPP_HIDE_FROM_ABI multimap(multimap&& __m) noexcept(is_nothrow_move_constructible<__base>::value)
- : __tree_(std::move(__m.__tree_)) {}
+ _LIBCPP_HIDE_FROM_ABI multimap(multimap&& __m) noexcept(is_nothrow_move_constructible<__base>::value) = default;
_LIBCPP_HIDE_FROM_ABI multimap(multimap&& __m, const allocator_type& __a);
- _LIBCPP_HIDE_FROM_ABI multimap& operator=(multimap&& __m) noexcept(is_nothrow_move_assignable<__base>::value) {
- __tree_ = std::move(__m.__tree_);
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI multimap&
+ operator=(multimap&& __m) noexcept(is_nothrow_move_assignable<__base>::value) = default;
_LIBCPP_HIDE_FROM_ABI multimap(initializer_list<value_type> __il, const key_compare& __comp = key_compare())
: __tree_(__vc(__comp)) {
diff --git a/libcxx/include/set b/libcxx/include/set
index aeea98adf582b..480aa0ca0b0c1 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -662,14 +662,10 @@ public:
_LIBCPP_HIDE_FROM_ABI set(const set& __s) : __tree_(__s.__tree_) { insert(__s.begin(), __s.end()); }
- _LIBCPP_HIDE_FROM_ABI set& operator=(const set& __s) {
- __tree_ = __s.__tree_;
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI set& operator=(const set& __s) = default;
# ifndef _LIBCPP_CXX03_LANG
- _LIBCPP_HIDE_FROM_ABI set(set&& __s) noexcept(is_nothrow_move_constructible<__base>::value)
- : __tree_(std::move(__s.__tree_)) {}
+ _LIBCPP_HIDE_FROM_ABI set(set&& __s) noexcept(is_nothrow_move_constructible<__base>::value) = default;
# endif // _LIBCPP_CXX03_LANG
_LIBCPP_HIDE_FROM_ABI explicit set(const allocator_type& __a) : __tree_(__a) {}
@@ -1129,14 +1125,10 @@ public:
insert(__s.begin(), __s.end());
}
- _LIBCPP_HIDE_FROM_ABI multiset& operator=(const multiset& __s) {
- __tree_ = __s.__tree_;
- return *this;
- }
+ _LIBCPP_HIDE_FROM_ABI multiset& operator=(const multiset& __s) = default;
# ifndef _LIBCPP_CXX03_LANG
- _LIBCPP_HIDE_FROM_ABI multiset(multiset&& __s) noexcept(is_nothrow_move_constructible<__base>::value)
- : __tree_(std::move(__s.__tree_)) {}
+ _LIBCPP_HIDE_FROM_ABI multiset(multiset&& __s) noexcept(is_nothrow_move_constructible<__base>::value) = default;
_LIBCPP_HIDE_FROM_ABI multiset(multiset&& __s, const allocator_type& __a);
# endif // _LIBCPP_CXX03_LANG
|
| # ifndef _LIBCPP_CXX03_LANG | ||
| __tree_ = __m.__tree_; | ||
| # else | ||
| if (this != std::addressof(__m)) { |
There was a problem hiding this comment.
It's not clear to me that before/after are equivalent in non-C++03 mode -- can you explain why they are?
Also, I had played around this area in https://reviews.llvm.org/D121485 (which never landed), can you take a look and ensure that concerns back then don't apply to this? Finally, can you benchmark before/after? In principle, we shouldn't see a difference IIUC?
There was a problem hiding this comment.
It's not clear to me that before/after are equivalent in non-C++03 mode -- can you explain why they are?
__tree_ is the only member of map, so the compiler-generated code just calls the assignment operator of __tree_.
Also, I had played around this area in https://reviews.llvm.org/D121485 (which never landed), can you take a look and ensure that concerns back then don't apply to this?
I guess Mark's comment in map, line 1902, is relevant here, but I don't think the concern is correct. We're calling __tree::clear() if we assign a new allocator and reuse the old nodes oterwise.
Finally, can you benchmark before/after?
I haven't benchmarked, but I have looked at the code gen before landing the patch, which was identical (even at -O0). Is that good enough?
In principle, we shouldn't see a difference IIUC?
Correct.
3 participants
We don't actually do anything special in these special member functions, so we can just
= defaultthem to save a bit of code.